Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix usage of onload = t.step_timeout in WPT tests #33034

Merged
merged 1 commit into from Mar 3, 2022

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Mar 2, 2022

t.step_timeout directly invokes setTimeout, so this pattern doesn't
actually wait for onload. The pattern, as used elsewhere, should be
something like:

onload = () => t.step_timeout

One app-history test in particular was causing issues as it became
dependent timer task ordering.

Bug: 1220223
Change-Id: I2a8c74e485833a3bc15cc53320c791ecb80452af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3499359
Auto-Submit: Scott Haseley <shaseley@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#976889}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

t.step_timeout directly invokes setTimeout, so this pattern doesn't
actually wait for onload. The pattern, as used elsewhere, should be
something like:

  onload = () => t.step_timeout

One app-history test in particular was causing issues as it became
dependent timer task ordering.

Bug: 1220223
Change-Id: I2a8c74e485833a3bc15cc53320c791ecb80452af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3499359
Auto-Submit: Scott Haseley <shaseley@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#976889}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants